Skip to content

test: deflake integration tests by polling instead of fixed sleeps#844

Merged
vdusek merged 6 commits into
masterfrom
test/fix-flaky-unlock-requests
Jun 4, 2026
Merged

test: deflake integration tests by polling instead of fixed sleeps#844
vdusek merged 6 commits into
masterfrom
test/fix-flaky-unlock-requests

Conversation

@vdusek
Copy link
Copy Markdown
Contributor

@vdusek vdusek commented Jun 2, 2026

Summary

Started as a fix for this flaky CI run of test_request_queue_unlock_requests[sync] (assert 2 == 3 on unlocked_count, caused by replication lag between list_and_lock_head and unlock_requests), and grew into deflaking the integration test suite as a whole.

Changes:

  • Add a poll_until_condition helper to tests/integration/_utils.py: polls a sync-or-async callable at a constant interval until a condition holds or a wall-clock timeout expires. An optional backoff_factor multiplies the interval after each poll, for highly variable waits (e.g. Actor run container startup) where a growing delay covers a long timeout with few calls.
  • Fix the flaky unlock test by polling list_head until the locked IDs disappear from the queue head before unlocking.
  • Replace all hand-rolled for _ in range(5): sleep(1); read; break polling loops (10×) and single fixed sleep(1) waits (26×) across the request queue, dataset, key-value store, and run integration tests with poll_until_condition. This makes the tests both faster on the happy path (no unconditional sleep) and more robust under load (polls until the timeout instead of hoping 1 s is enough).
  • Generalize maybe_await to accept any awaitable.

The three iterate_keys sleeps in the KVS tests are intentionally left as-is: draining an iterator per attempt wants attempt-count semantics (like collect_iterate_until_present), not a wall-clock deadline.

Follow-up to #786.

@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Jun 2, 2026
@vdusek vdusek self-assigned this Jun 2, 2026
@github-actions github-actions Bot added this to the 142nd sprint - Tooling team milestone Jun 2, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Jun 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.82%. Comparing base (d2ab2ae) to head (c856ab1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #844      +/-   ##
==========================================
+ Coverage   94.68%   94.82%   +0.13%     
==========================================
  Files          48       48              
  Lines        5045     5045              
==========================================
+ Hits         4777     4784       +7     
+ Misses        268      261       -7     
Flag Coverage Δ
integration 93.24% <ø> (+0.23%) ⬆️
unit 83.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek vdusek changed the base branch from master to test/unify-polling-helpers June 2, 2026 13:44
@vdusek vdusek changed the base branch from test/unify-polling-helpers to master June 2, 2026 13:53
@vdusek vdusek force-pushed the test/fix-flaky-unlock-requests branch from 44e03e0 to ad43eec Compare June 2, 2026 13:53
vdusek added 2 commits June 3, 2026 12:05
Add a wall-clock-deadline `poll_until_condition` helper, generalize `maybe_await` to any awaitable, and refactor `collect_iterate_until_present` to reuse a shared drain step.
Lock writes propagate asynchronously after `list_and_lock_head` returns, so unlocking immediately could see fewer locks than acquired; poll `list_head` until the locked IDs disappear from the queue head before unlocking.
@vdusek vdusek force-pushed the test/fix-flaky-unlock-requests branch from 1cb554e to e23e54d Compare June 3, 2026 10:06
@vdusek vdusek changed the title test: fix flaky test_request_queue_unlock_requests test: deflake integration tests by polling instead of fixed sleeps Jun 3, 2026
@vdusek vdusek requested a review from janbuchar June 3, 2026 10:41
Comment thread tests/integration/test_run.py
@vdusek vdusek merged commit 6c68c8b into master Jun 4, 2026
27 checks passed
@vdusek vdusek deleted the test/fix-flaky-unlock-requests branch June 4, 2026 09:55
vdusek added a commit to apify/crawlee-python that referenced this pull request Jun 4, 2026
…#1926)

Unifies eventually-consistent / settling waits across the test suite
behind a single `poll_until_condition` helper, matching the form shared
with apify/apify-client-python#844.

## Changes

- **Single helper.** Replace `wait_for_condition` with
`poll_until_condition` (`tests/_utils.py`): it polls a sync-or-async
callable until an optional `condition` holds (default: truthy) or a
wall-clock `timeout` expires, then returns the last result so the caller
asserts explicitly. Exponential backoff is folded into a
`backoff_factor` argument instead of a separate `call_with_exp_backoff`.
- **Shared location + relative imports.** Move the helper from
`tests/unit/utils.py` to the shared `tests/_utils.py` and import it
relatively. This requires the test tree to be a proper package, so add
`__init__.py` to the `tests/unit` and `tests/e2e` subpackages that
lacked one (and allow `TID252` in tests).
- **Migrate hand-rolled waits.** Replace fixed `sleep`s and
`for`/`while` polling loops with the helper:
  - 5 autoscaling waits — `test_autoscaled_pool.py`
  - statistics-initialization loop — `test_basic_crawler.py`
  - 2 sitemap-loader waits — `test_sitemap_request_loader.py`
  - 2 request-queue background-task waits — `test_request_queue.py`

Lint, type-check, and the full unit suite (1883 passed) pass.
vdusek added a commit to apify/apify-sdk-python that referenced this pull request Jun 5, 2026
…flake via polling (#920)

## Summary

The SDK counterpart of apify/apify-client-python#844: introduce a single
shared `tests/_utils.py` with one polling helper, and migrate the whole
test suite to it — replacing fixed sleeps and hand-rolled retry loops
that caused flakiness.

## Changes

- **Shared `tests/_utils.py`.** One `poll_until_condition(fn,
condition=bool, *, timeout, poll_interval, backoff_factor)` helper —
identical to the one in apify-client-python#844 — polls a sync-or-async
callable until a condition holds or a wall-clock timeout expires.
`backoff_factor=2` subsumes the former `call_with_exp_backoff`;
`timeout=0` is the "call once" case. The `maybe_await` adapter,
`generate_unique_resource_name`, and the shared RSA crypto test keys
move here too. The per-package `tests/integration/_utils.py` and
`tests/e2e/_utils.py` are removed, and cross-test imports (e.g. the
crypto keys, previously imported from the `test_crypto` module) now go
through this single module.
- **Request queue tests.** ~50 polling call sites in
`test_request_queue.py` migrated to `poll_until_condition`. The
single/shared timeout is centralized in an `rq_poll_timeout` fixture
(`timeout=0` single, `timeout=30` shared), backed by an `rq_access_mode`
fixture — replacing the access-mode derivation block previously
copy-pasted into every test. The shared-mode request-ordering
relaxations from #931 are preserved on top.
- **Deflaked tests.** Fixed the flaky
`test_actor_adds_webhook_and_receives_event` e2e test (the client now
stays alive 5 s after `add_webhook`, and the unbounded `INITIALIZED`
startup loop becomes bounded polling) — replaces #930. Replaced the
`retry_counter` loops in `test_actor_charge.py` (4×) and fixed sleeps in
`test_actor_lifecycle`, `test_actor_key_value_store`, and
`test_apify_event_manager` with condition polling.

Fixed sleeps that are semantically required are intentionally kept:
negative checks ("event must NOT fire"), mtime-granularity checks,
simulated latency, and sleeps inside deployed Actor `main` bodies (where
the helper is unavailable).

Verified: lint, type-check, and the full unit suite pass;
integration/e2e require a platform token and run in CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants